-
-
Notifications
You must be signed in to change notification settings - Fork 435
test: correct lightclient committee root test for electra #8825
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: unstable
Are you sure you want to change the base?
test: correct lightclient committee root test for electra #8825
Conversation
Summary of ChangesHello @lodekeeper, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical issue in the light client committee root end-to-end test that emerged following the Electra upgrade. The previous test logic incorrectly assumed a simple alternating pattern for sync committee public keys, which is not how committees are formed in practice, especially with the changes introduced in Electra. The fix updates the test to dynamically retrieve the actual sync committee root from the beacon node's head state, ensuring the test accurately reflects the network's behavior and passes reliably. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request provides a solid fix for a failing lightclient e2e test that occurred after the Electra upgrade. The root cause was an incorrect assumption in the test about how sync committees are constructed. The new implementation correctly fetches the expected committee root directly from the head state, making the test more robust and accurate. The code is clean, the change is well-contained, and the removal of now-unnecessary code and imports is a good cleanup. Overall, this is an excellent contribution.
The test incorrectly assumed sync committees would have alternating pubkeys [pk0, pk1, pk0, pk1, ...], but sync committees are computed using a weighted random shuffle based on validator effective balances and a seed. Post-electra, the shuffle algorithm uses 16-bit random values (vs 8-bit pre-electra) and MAX_EFFECTIVE_BALANCE_ELECTRA, producing different committee distributions. Fix: Get the actual sync committee root from the head state instead of constructing an incorrect expected committee. Closes ChainSafe#8723
4107e79 to
99941e8
Compare
Description
Fixes the failing lightclient e2e test after upgrading to electra.
Root Cause
The test incorrectly assumed sync committees would have alternating pubkeys
[pk0, pk1, pk0, pk1, ...]:However, sync committees are computed using a weighted random shuffle based on:
Why it broke post-electra
In
getNextSyncCommitteeIndices(), the shuffle parameters changed for electra:The shuffle algorithm now uses 2 random bytes instead of 1, producing a completely different committee distribution even with the same validators.
Fix
Get the actual sync committee root from the head state instead of constructing an incorrect expected committee.
Closes #8723
Note
This PR was authored by Lodekeeper (AI assistant) under supervision of @nflaig.